Deploy an oracle cronjob #1814

yerdua · 2019-11-20T18:22:34Z

Description

Helm chart and celotool command to deploy an oracle cronjob to a network.

Note: In order to get this working for baklavastaging, I merged in @tkporter's branch trevor/baklavastaging-nov-21. So, that branch should be merged into master first. Or this one should be merged into that one instead.

Tested

Tested by running the celotool command to deploy an oracle to baklavastaging, and confirming that the rates are reported to the network correctly.

Other Changes

Update contractkit's version to accurately reflect what's been published on npm

Related issues

Fixes CLabs SBAT operate a single oracle with Brownian motion #981

yerdua · 2019-11-20T18:27:24Z

.env

@@ -32,6 +32,12 @@ GETH_BOOTNODE_DOCKER_IMAGE_TAG="master"
 CELOTOOL_DOCKER_IMAGE_REPOSITORY="gcr.io/celo-testnet/celo-monorepo"
 CELOTOOL_DOCKER_IMAGE_TAG="celotool-dfdc3e8b26e98aa294b27e2b5621c184488a10db"

+CELOCLI_STANDALONE_IMAGE_REPOSITORY="gcr.io/celo-testnet/celocli-standalone"
+# TODO (yerdua): Change this to something actually reasonable
+CELOCLI_STANDALONE_IMAGE_TAG="yerduaoracletest"


This really doesn't seem ideal. I needed a docker image that can run the cli. The existing dockerfile has deeply hardcoded references to alfajores and runs a full node. In order to get something here, I built an image and pushed it manually.

Could you add both env vars to all .env files?

yerdua · 2019-11-20T18:56:26Z

There's a few incomplete things in this PR. The primary one is the handling of the docker image for celocli. I've merged the needed cli changes into master, but they're not yet published on npm. How best to handle tagging, versions, and the docker image building are all kind of opaque to me, so I'd appreciate some advice on how to do those right.

Also, there is no default oracle process. There's a docker image in a private repo, for my testnet, and for baklava. In an ideal world, there should probably be some default image, maybe just spitting out the exchange rate value specified in the config. Given the tight deadline, I think it's ok if the command fails when there's no oracle image for now.

tkporter

Couple small changes pending the new cli version

tkporter · 2019-11-21T10:25:34Z

packages/celotool/src/lib/oracle.ts

+
+function helmParameters(celoEnv: string) {
+  return [
+    `--set celotool.image.repository=${fetchEnv('CELOTOOL_DOCKER_IMAGE_REPOSITORY')}`,


nit: some of these string literals are already found in envVar, if they aren't we should add them and use them to avoid hardcoding strings

tkporter · 2019-11-21T10:44:18Z

packages/celotool/src/lib/oracle.ts

+    `--set celotool.image.repository=${fetchEnv('CELOTOOL_DOCKER_IMAGE_REPOSITORY')}`,
+    `--set celotool.image.tag=${fetchEnv('CELOTOOL_DOCKER_IMAGE_TAG')}`,
+    `--set mnemonic="${fetchEnv(envVar.MNEMONIC)}"`,
+    `--set oracle.image.repository=${fetchEnv('ORACLE_DOCKER_IMAGE_REPOSITORY')}`,


is this used? same goes for any use of ORACLE_DOCKER_IMAGE_REPOSITORY

tkporter · 2019-11-21T10:45:08Z

.env

@@ -32,6 +32,12 @@ GETH_BOOTNODE_DOCKER_IMAGE_TAG="master"
 CELOTOOL_DOCKER_IMAGE_REPOSITORY="gcr.io/celo-testnet/celo-monorepo"
 CELOTOOL_DOCKER_IMAGE_TAG="celotool-dfdc3e8b26e98aa294b27e2b5621c184488a10db"

+CELOCLI_STANDALONE_IMAGE_REPOSITORY="gcr.io/celo-testnet/celocli-standalone"
+# TODO (yerdua): Change this to something actually reasonable
+CELOCLI_STANDALONE_IMAGE_TAG="yerduaoracletest"


Could you add both env vars to all .env files?

tkporter · 2019-11-21T10:46:59Z

packages/helm-charts/oracle/templates/oracle.cronjob.yaml

+        spec:
+          initContainers:
+          - name: get-current-price
+            image: gcr.io/celo-testnet/oracle:{{ .Release.Namespace }}


Ah I think I get it now- should we set the repository and tag here similar to what we do for geth or celotool by having it depend on .env variables?

Oops! Thank you! This is where those unused envVars were supposed to go.

tkporter · 2019-11-21T10:48:18Z

packages/helm-charts/oracle/values.yaml

+nodeSelector: {}
+
+# Expects cron schedule syntax
+oracleSchedule: "*/1 * * * *"


might be helpful to add a comment what this schedule means in readable terms for people like myself that never remember the cron schedule structure

Excellent idea. I had to look it up myself.
Do you think this makes sense here, or should it be an envVar like all the other ones?

codecov · 2019-11-26T00:12:51Z

Codecov Report

❗ No coverage uploaded for pull request base (master@8f87dda). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1814   +/-   ##
=========================================
  Coverage          ?   74.45%           
=========================================
  Files             ?      281           
  Lines             ?     7817           
  Branches          ?      973           
=========================================
  Hits              ?     5820           
  Misses            ?     1880           
  Partials          ?      117

Flag	Coverage Δ
#mobile	`74.45% <ø> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f87dda...2f03d67. Read the comment docs.

yerdua · 2019-11-26T00:15:19Z

Note: This branch has trevor/baklavastaging-nov-21 merged into it so deployment could work.

jmrossy

Suggest adding a note at the top of the PR description that Trevor's branch changes must merge first before this :)

…ge without geth

…lotool deployment command for the oracle

…y exists

…sses come that way

yerdua · 2019-12-02T19:23:13Z

.env.baklavastaging

+CELOCLI_STANDALONE_IMAGE_REPOSITORY="gcr.io/celo-testnet/celocli-standalone"
+CELOCLI_STANDALONE_IMAGE_TAG="0.0.30-beta2"
+
+# Schedule for an oracle deployed via celotool, expressed in crontab syntax
+# This schedule is "every 5th minute"
+ORACLE_CRON_SCHEDULE="*/5 * * * *"
+
+ORACLE_DOCKER_IMAGE_REPOSITORY="gcr.io/celo-testnet/oracle"
+ORACLE_DOCKER_IMAGE_TAG="baklavastaging"


The origin of this file is @tkporter's branch for baklavastaging deployment, as it is today (Dec 2). These vars are the only ones I added:

CELOCLI_STANDALONE_IMAGE_REPOSITORY CELOCLI_STANDALONE_IMAGE_TAG ORACLE_CRON_SCHEDULE ORACLE_DOCKER_IMAGE_REPOSITORY ORACLE_DOCKER_IMAGE_TAG

So, if there are merge conflicts in this file that do not involve these variables, the resolution is to pick whatever the other branch has.

The only one of these that should be different for baklava is:
ORACLE_DOCKER_IMAGE_TAG="baklava"

That docker image already exists and is ready to go.

yerdua · 2019-12-02T19:26:59Z

packages/protocol/migrations/08_stabletoken.ts

@@ -46,7 +47,7 @@ module.exports = deploymentForCoreContract<StableTokenInstance>(

    for (const oracle of config.stableToken.oracles) {
      console.info(`Adding ${oracle} as an Oracle for StableToken`)
-      await sortedOracles.addOracle(stableToken.address, oracle)
+      await sortedOracles.addOracle(stableToken.address, ensureHexLeader(oracle))


Without the 0x, the oracle silently doesn't get whitelisted correctly. This needs to be merged before baklava deployment.

tkporter

Super minor comments, nice!

tkporter · 2019-12-02T19:37:45Z

packages/celotool/src/cmds/deploy/destroy/oracle.ts

+export const builder = {}
+
+export const handler = async (argv: DestroyArgv) => {
+  await createClusterIfNotExists()


You should be able to remove this line to decrease the run time-- if we're destroying we should assume that the cluster exists & if it doesn't there's no need in creating it

tkporter · 2019-12-02T19:55:08Z

packages/helm-charts/oracle/templates/oracle.cronjob.yaml

+              - sh
+              - "-c"
+              - |
+                echo `./current_rate.sh` > /celo/.celo/current_price


nit: should be able to just run ./current_rate.sh > /celo/.celo/current_price right?

* master: Update docker image and instructions (#2000) DOCS correct typos (#1965) Add a space on payment request (#1935) expose signer through getValidator (#1997) Fix validator election migration bug with sorted list insertion (#1998) Deploy an oracle cronjob (#1814) Set default env in attestation service docker image as production (#1999) Attestation Bot POC (#1851) Only allow external RPCs for tx nodes (#1994)

* master: (120 commits) Update docker image and instructions (#2000) DOCS correct typos (#1965) Add a space on payment request (#1935) expose signer through getValidator (#1997) Fix validator election migration bug with sorted list insertion (#1998) Deploy an oracle cronjob (#1814) Set default env in attestation service docker image as production (#1999) Attestation Bot POC (#1851) Only allow external RPCs for tx nodes (#1994) Specify web deps properly (#1950) Fix error calculating fees on currencies with high exchange rate (#1937) Small pre-stake-off CLI cleanup (#1953) Migrations: different CLabs groups gets different votes (#1960) [Wallet] Fix broken translation (decline, pay) (#1968) [Wallet]: Add Payments You've Requested notifications group & screen (#1902) [Wallet] Fix iOS bundle script failing with our monorepo setup (#1958) [NotificationService] Change exchange rate stored format (number timestamp and pair) (#1945) [Wallet] Fix premature hiding of syncing banner and increase some timeouts (#1957) Avoid `liner: function not supported in this terminal` (#1955) Support more human readable log output (#1929) ...

yerdua requested review from nambrot and timmoreton as code owners November 20, 2019 18:22

yerdua commented Nov 20, 2019

View reviewed changes

yerdua assigned yerdua and tkporter Nov 20, 2019

timmoreton added the betanet-blocker label Nov 21, 2019

tkporter reviewed Nov 21, 2019

View reviewed changes

yerdua requested review from asaj, cmcewen, jmrossy and mcortesi as code owners November 26, 2019 00:12

yerdua changed the base branch from master to trevor/baklavastaging-nov-21 November 26, 2019 00:19

yerdua changed the base branch from trevor/baklavastaging-nov-21 to master November 26, 2019 00:19

jmrossy reviewed Nov 28, 2019

View reviewed changes

yerdua changed the base branch from master to trevor/baklavastaging-nov-21 November 28, 2019 13:57

yerdua changed the base branch from trevor/baklavastaging-nov-21 to master November 28, 2019 13:59

yerdua added 10 commits December 2, 2019 13:14

add helm chart for oracle cronjob

f12411f

celotool command for deploying an oracle job (WIP)

f3c0616

more helm chart work

e7710d4

move some values into .env. add a dockerfile for a standalone cli ima…

19f0e69

…ge without geth

deal with some envVars in a better way

9600fe1

hardcode a node's IP because forno is broken. fix some bits of the ce…

cb319f6

…lotool deployment command for the oracle

various things to make the oracle actually work

25e7668

get rid of temporary hardcode now that forno works

e95cba0

yarn.lock cleanup via yarn install

9e4e9e5

make oracle schedule be in the .env file

75d9681

yerdua force-pushed the yerdua/oracle-cronjob branch from 60f0540 to 75d9681 Compare December 2, 2019 15:59

yerdua added 2 commits December 2, 2019 17:05

remove cli changes from this PR, since they're not in published cli

35e8d51

fix contractkit dependency for cli

5b202a0

yerdua removed their assignment Dec 2, 2019

yerdua added 4 commits December 2, 2019 18:12

add the new envvars to all .envs. Use a cli docker image that actuall…

61b8a6e

…y exists

addOracle expects leading '0x' on the address. Don't assume the addre…

92ad324

…sses come that way

add destroy oracle command

3cda3c7

Merge branch 'master' into yerdua/oracle-cronjob

3f137d6

yerdua requested a review from m-chrzan as a code owner December 2, 2019 19:12

get latest .env.baklavastaging from trevor's branch

d1d5e8e

yerdua commented Dec 2, 2019

View reviewed changes

oops. actually include the necessary bits of the destroy oracle command

ce6a931

tkporter approved these changes Dec 2, 2019

View reviewed changes

fix minor issues

ced08ae

yerdua added the automerge Have PR merge automatically when checks pass label Dec 2, 2019

tkporter added 3 commits December 2, 2019 15:07

Merge branch 'master' into yerdua/oracle-cronjob

706eaf4

Merge branch 'master' into yerdua/oracle-cronjob

60fcf27

Merge branch 'master' into yerdua/oracle-cronjob

2f03d67

celo-ci-bot-user merged commit cd2a2e2 into master Dec 3, 2019

celo-ci-bot-user deleted the yerdua/oracle-cronjob branch December 3, 2019 01:41

leojoy95 mentioned this pull request Oct 24, 2024

[Snyk] Fix for 58 vulnerabilities leojoy95/celo-monorepo#1271

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy an oracle cronjob #1814

Deploy an oracle cronjob #1814

yerdua commented Nov 20, 2019 •

edited

Loading

yerdua Nov 20, 2019

tkporter Nov 21, 2019

yerdua commented Nov 20, 2019

tkporter left a comment

tkporter Nov 21, 2019

tkporter Nov 21, 2019

tkporter Nov 21, 2019

tkporter Nov 21, 2019

yerdua Nov 21, 2019

tkporter Nov 21, 2019

yerdua Nov 21, 2019

codecov bot commented Nov 26, 2019 •

edited

Loading

yerdua commented Nov 26, 2019

jmrossy left a comment

yerdua Dec 2, 2019 •

edited

Loading

yerdua Dec 2, 2019

yerdua Dec 2, 2019

tkporter left a comment

tkporter Dec 2, 2019

tkporter Dec 2, 2019

Deploy an oracle cronjob #1814

Deploy an oracle cronjob #1814

Conversation

yerdua commented Nov 20, 2019 • edited Loading

Description

Tested

Other Changes

Related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yerdua commented Nov 20, 2019

tkporter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 26, 2019 • edited Loading

Codecov Report

yerdua commented Nov 26, 2019

jmrossy left a comment

Choose a reason for hiding this comment

yerdua Dec 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkporter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yerdua commented Nov 20, 2019 •

edited

Loading

codecov bot commented Nov 26, 2019 •

edited

Loading

yerdua Dec 2, 2019 •

edited

Loading